Make SourceId cheap to clone
authorAlex Crichton <alex@alexcrichton.com>
Fri, 17 Oct 2014 18:48:57 +0000 (11:48 -0700)
committerAlex Crichton <alex@alexcrichton.com>
Wed, 22 Oct 2014 18:29:52 +0000 (11:29 -0700)
Like PackageId, these are cloned quite often, so this moves them into an Arc to
make them cheap to clone. This also removes the public fields in favor of
accessors.

src/cargo/core/package_id_spec.rs
src/cargo/core/source.rs
src/cargo/ops/cargo_rustc/fingerprint.rs
src/cargo/sources/git/source.rs

index bfce8cd1e1d7114cd05a4d005e010ce5a95b6a13..7ce03616d9007a1ad1f2b8949c0ceedc451295d0 100644 (file)
@@ -49,7 +49,7 @@ impl PackageIdSpec {
         PackageIdSpec {
             name: package_id.get_name().to_string(),
             version: Some(package_id.get_version().clone()),
-            url: Some(package_id.get_source_id().url.clone()),
+            url: Some(package_id.get_source_id().get_url().clone()),
         }
     }
 
@@ -110,7 +110,7 @@ impl PackageIdSpec {
         }
 
         match self.url {
-            Some(ref u) => *u == package_id.get_source_id().url,
+            Some(ref u) => u == package_id.get_source_id().get_url(),
             None => true
         }
     }
index 6b029b7008b5b0dd0c6a07b342d657055ab77c50..1e6df19d506fee7b755ff1477b7085dedbe570fc 100644 (file)
@@ -5,6 +5,7 @@ use std::fmt::{mod, Show, Formatter};
 use std::hash;
 use std::iter;
 use std::mem;
+use std::sync::Arc;
 use serialize::{Decodable, Decoder, Encodable, Encoder};
 
 use url::Url;
@@ -59,104 +60,26 @@ type Error = Box<CargoError + Send>;
 /// Unique identifier for a source of packages.
 #[deriving(Clone, Eq)]
 pub struct SourceId {
-    pub url: Url,
-    pub kind: SourceKind,
-    // e.g. the exact git revision of the specified branch for a Git Source
-    pub precise: Option<String>
-}
-
-impl PartialOrd for SourceId {
-    fn partial_cmp(&self, other: &SourceId) -> Option<Ordering> {
-        self.to_string().partial_cmp(&other.to_string())
-    }
-}
-
-impl Ord for SourceId {
-    fn cmp(&self, other: &SourceId) -> Ordering {
-        self.to_string().cmp(&other.to_string())
-    }
-}
-
-impl<E, S: Encoder<E>> Encodable<S, E> for SourceId {
-    fn encode(&self, s: &mut S) -> Result<(), E> {
-        if self.is_path() {
-            s.emit_option_none()
-        } else {
-           self.to_url().encode(s)
-        }
-    }
-}
-
-impl<E, D: Decoder<E>> Decodable<D, E> for SourceId {
-    fn decode(d: &mut D) -> Result<SourceId, E> {
-        let string: String = Decodable::decode(d).ok().expect("Invalid encoded SourceId");
-        Ok(SourceId::from_url(string))
-    }
-}
-
-impl Show for SourceId {
-    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
-        match *self {
-            SourceId { kind: PathKind, ref url, .. } => url.fmt(f),
-            SourceId { kind: GitKind(ref reference), ref url, ref precise, .. } => {
-                try!(write!(f, "{}", url));
-                if reference.as_slice() != "master" {
-                    try!(write!(f, "?ref={}", reference));
-                }
-
-                match *precise {
-                    Some(ref s) => {
-                        try!(write!(f, "#{}", s.as_slice().slice_to(8)));
-                    }
-                    None => {}
-                }
-                Ok(())
-            },
-            SourceId { kind: RegistryKind, ref url, .. } => {
-                let default = RegistrySource::url().ok();
-                if default.as_ref() == Some(url) {
-                    write!(f, "the package registry")
-                } else {
-                    write!(f, "registry {}", url)
-                }
-            }
-        }
-    }
+    inner: Arc<SourceIdInner>,
 }
 
-// This custom implementation handles situations such as when two git sources
-// point at *almost* the same URL, but not quite, even when they actually point
-// to the same repository.
-impl PartialEq for SourceId {
-    fn eq(&self, other: &SourceId) -> bool {
-        if self.kind != other.kind { return false }
-        if self.url == other.url { return true }
-
-        match (&self.kind, &other.kind, &self.url, &other.url) {
-            (&GitKind(ref ref1), &GitKind(ref ref2), u1, u2) => {
-                ref1 == ref2 &&
-                git::canonicalize_url(u1) == git::canonicalize_url(u2)
-            }
-            _ => false,
-        }
-    }
-}
-
-impl<S: hash::Writer> hash::Hash<S> for SourceId {
-    fn hash(&self, into: &mut S) {
-        self.kind.hash(into);
-        match *self {
-            SourceId { kind: GitKind(..), ref url, .. } => {
-                git::canonicalize_url(url).hash(into)
-            }
-            _ => self.url.hash(into),
-        }
-    }
+#[deriving(Eq, Clone)]
+struct SourceIdInner {
+    url: Url,
+    kind: SourceKind,
+    // e.g. the exact git revision of the specified branch for a Git Source
+    precise: Option<String>
 }
 
 impl SourceId {
     pub fn new(kind: SourceKind, url: Url) -> SourceId {
-        SourceId { kind: kind, url: url, precise: None }
+        SourceId {
+            inner: Arc::new(SourceIdInner {
+                kind: kind,
+                url: url,
+                precise: None,
+            }),
+        }
     }
 
     /// Parses a source URL and returns the corresponding ID.
@@ -198,12 +121,12 @@ impl SourceId {
     }
 
     pub fn to_url(&self) -> String {
-        match *self {
-            SourceId { kind: PathKind, .. } => {
+        match *self.inner {
+            SourceIdInner { kind: PathKind, .. } => {
                 fail!("Path sources are not included in the lockfile, \
                        so this is unimplemented")
             },
-            SourceId {
+            SourceIdInner {
                 kind: GitKind(ref reference), ref url, ref precise, ..
             } => {
                 let ref_str = if reference.as_slice() != "master" {
@@ -220,7 +143,7 @@ impl SourceId {
 
                 format!("git+{}{}{}", url, ref_str, precise_str)
             },
-            SourceId { kind: RegistryKind, .. } => {
+            SourceIdInner { kind: RegistryKind, .. } => {
                 // TODO: Central registry vs. alternates
                 "registry+https://crates.io/".to_string()
             }
@@ -255,15 +178,19 @@ impl SourceId {
     }
 
     pub fn get_url(&self) -> &Url {
-        &self.url
+        &self.inner.url
+    }
+
+    pub fn get_kind(&self) -> &SourceKind {
+        &self.inner.kind
     }
 
     pub fn is_path(&self) -> bool {
-        self.kind == PathKind
+        self.inner.kind == PathKind
     }
 
     pub fn is_git(&self) -> bool {
-        match self.kind {
+        match self.inner.kind {
             GitKind(_) => true,
             _ => false
         }
@@ -272,10 +199,10 @@ impl SourceId {
     /// Creates an implementation of `Source` corresponding to this ID.
     pub fn load<'a>(&self, config: &'a mut Config) -> Box<Source+'a> {
         log!(5, "loading SourceId; {}", self);
-        match self.kind {
+        match self.inner.kind {
             GitKind(..) => box GitSource::new(self, config) as Box<Source+'a>,
             PathKind => {
-                let path = match self.url.to_file_path() {
+                let path = match self.inner.url.to_file_path() {
                     Ok(p) => p,
                     Err(()) => fail!("path sources cannot be remote"),
                 };
@@ -285,10 +212,112 @@ impl SourceId {
         }
     }
 
+    pub fn get_precise(&self) -> Option<&str> {
+        self.inner.precise.as_ref().map(|s| s.as_slice())
+    }
+
     pub fn with_precise(&self, v: String) -> SourceId {
         SourceId {
-            precise: Some(v),
-            .. self.clone()
+            inner: Arc::new(SourceIdInner {
+                precise: Some(v),
+                .. (*self.inner).clone()
+            }),
+        }
+    }
+}
+
+impl PartialEq for SourceId {
+    fn eq(&self, other: &SourceId) -> bool {
+        self.inner.eq(&*other.inner)
+    }
+}
+
+impl PartialOrd for SourceId {
+    fn partial_cmp(&self, other: &SourceId) -> Option<Ordering> {
+        self.to_string().partial_cmp(&other.to_string())
+    }
+}
+
+impl Ord for SourceId {
+    fn cmp(&self, other: &SourceId) -> Ordering {
+        self.to_string().cmp(&other.to_string())
+    }
+}
+
+impl<E, S: Encoder<E>> Encodable<S, E> for SourceId {
+    fn encode(&self, s: &mut S) -> Result<(), E> {
+        if self.is_path() {
+            s.emit_option_none()
+        } else {
+           self.to_url().encode(s)
+        }
+    }
+}
+
+impl<E, D: Decoder<E>> Decodable<D, E> for SourceId {
+    fn decode(d: &mut D) -> Result<SourceId, E> {
+        let string: String = Decodable::decode(d).ok().expect("Invalid encoded SourceId");
+        Ok(SourceId::from_url(string))
+    }
+}
+
+impl Show for SourceId {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+        match *self.inner {
+            SourceIdInner { kind: PathKind, ref url, .. } => url.fmt(f),
+            SourceIdInner { kind: GitKind(ref reference), ref url,
+                            ref precise, .. } => {
+                try!(write!(f, "{}", url));
+                if reference.as_slice() != "master" {
+                    try!(write!(f, "?ref={}", reference));
+                }
+
+                match *precise {
+                    Some(ref s) => {
+                        try!(write!(f, "#{}", s.as_slice().slice_to(8)));
+                    }
+                    None => {}
+                }
+                Ok(())
+            },
+            SourceIdInner { kind: RegistryKind, ref url, .. } => {
+                let default = RegistrySource::url().ok();
+                if default.as_ref() == Some(url) {
+                    write!(f, "the package registry")
+                } else {
+                    write!(f, "registry {}", url)
+                }
+            }
+        }
+    }
+}
+
+// This custom implementation handles situations such as when two git sources
+// point at *almost* the same URL, but not quite, even when they actually point
+// to the same repository.
+impl PartialEq for SourceIdInner {
+    fn eq(&self, other: &SourceIdInner) -> bool {
+        if self.kind != other.kind { return false }
+        if self.url == other.url { return true }
+
+        match (&self.kind, &other.kind, &self.url, &other.url) {
+            (&GitKind(ref ref1), &GitKind(ref ref2), u1, u2) => {
+                ref1 == ref2 &&
+                git::canonicalize_url(u1) == git::canonicalize_url(u2)
+            }
+            _ => false,
+        }
+    }
+}
+
+impl<S: hash::Writer> hash::Hash<S> for SourceId {
+    fn hash(&self, into: &mut S) {
+        self.inner.kind.hash(into);
+        match *self.inner {
+            SourceIdInner { kind: GitKind(..), ref url, .. } => {
+                git::canonicalize_url(url).hash(into)
+            }
+            _ => self.inner.url.hash(into),
         }
     }
 }
index 3f6f4e6a316288458bac05ca1dfa29b619d818bb..4027d3a2fbfe83c29fb936eaee27b21b7affffb0 100644 (file)
@@ -54,7 +54,7 @@ pub fn prepare_target(cx: &mut Context, pkg: &Package, target: &Target,
     // constant (which is the responsibility of the source)
     let use_pkg = {
         let doc = target.get_profile().is_doc();
-        let path = match pkg.get_summary().get_source_id().kind {
+        let path = match *pkg.get_summary().get_source_id().get_kind() {
             PathKind => true,
             _ => false,
         };
index b68a6e9ef837c0540c889b15b6c553fc76a30773..8938bac559973d96099b5283945e5bb3f6365cec 100644 (file)
@@ -28,7 +28,7 @@ impl<'a, 'b> GitSource<'a, 'b> {
                        config: &'a mut Config<'b>) -> GitSource<'a, 'b> {
         assert!(source_id.is_git(), "id is not git, id={}", source_id);
 
-        let reference = match source_id.kind {
+        let reference = match *source_id.get_kind() {
             GitKind(ref reference) => reference,
             _ => fail!("Not a git source; id={}", source_id)
         };
@@ -42,9 +42,9 @@ impl<'a, 'b> GitSource<'a, 'b> {
         let checkout_path = config.git_checkout_path()
             .join(ident.as_slice()).join(reference.as_slice());
 
-        let reference = match source_id.precise {
-            Some(ref s) => s,
-            None => reference,
+        let reference = match source_id.get_precise() {
+            Some(s) => s,
+            None => reference.as_slice(),
         };
 
         GitSource {
@@ -160,7 +160,7 @@ impl<'a, 'b> Source for GitSource<'a, 'b> {
         let actual_rev = self.remote.rev_for(&self.db_path,
                                              self.reference.as_slice());
         let should_update = actual_rev.is_err() ||
-                            self.source_id.precise.is_none();
+                            self.source_id.get_precise().is_none();
 
         let (repo, actual_rev) = if should_update {
             try!(self.config.shell().status("Updating",